Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds unhandled exception logging in the global exception handler, attempts to gracefully handle missing payments during reservation cancellation, cleans up stale payments during reservation updates, and removes unsupported sync caching in StoreService. However, the review identified two issues in ReservationService: first, catching a runtime exception during refunding still marks the transaction as rollback-only, which will cause an UnexpectedRollbackException on commit; second, checking for existing payments using a newly created reservation's ID is redundant dead code that triggers an unnecessary database query.
| try { | ||
| paymentService.refundPayment(reservation); | ||
| } catch (CustomException e) { | ||
| if (e.getErrorCode() != ErrorCode.PAYMENT_NOT_FOUND) { | ||
| throw e; | ||
| } | ||
| log.warn("환불 대상 결제 없음 — 취소 진행: reservationId={}", reservationId); | ||
| } |
There was a problem hiding this comment.
Spring의 @Transactional 동작 방식 때문에 발생하는 심각한 버그가 있습니다.
cancelReservation 메서드와 paymentService.refundPayment 메서드 모두 @Transactional이 적용되어 있어 동일한 트랜잭션 범위를 공유합니다. refundPayment 내에서 CustomException (RuntimeException)이 발생하면, 호출부인 cancelReservation에서 이를 try-catch로 잡아내더라도 이미 해당 트랜잭션은 rollback-only 상태로 마킹됩니다.
결과적으로 cancelReservation 메서드가 정상적으로 종료되는 시점에 UnexpectedRollbackException이 발생하며 전체 트랜잭션이 롤백되어, 예약 취소 및 재고 복구 로직이 모두 실패하게 됩니다.
이를 해결하기 위해 예외를 발생시키고 잡는 대신, ReservationService에서 paymentRepository를 통해 결제 정보 존재 여부를 사전에 체크한 후 refundPayment를 호출하도록 변경하는 것이 안전합니다.
if (paymentRepository.findByReservation_Id(reservation.getId()).isPresent()) {
paymentService.refundPayment(reservation);
} else {
log.warn("환불 대상 결제 없음 — 취소 진행: reservationId={}", reservationId);
}| paymentRepository.findByReservation_Id(newReservation.getId()).ifPresent(stale -> { | ||
| if (stale.getStatus() == PaymentStatus.PAID) { | ||
| throw new CustomException(ErrorCode.PAYMENT_ALREADY_PAID); | ||
| } | ||
| paymentRepository.delete(stale); | ||
| paymentRepository.flush(); | ||
| }); |
There was a problem hiding this comment.
newReservation은 바로 직전 라인(316번째 줄)에서 createReservationCore를 통해 새롭게 생성되어 저장된 예약 엔티티입니다.
따라서 데이터베이스에 이 새로운 예약 ID(newReservation.getId())와 매핑된 기존 결제(Payment) 정보가 존재할 가능성은 전혀 없습니다. 결과적으로 paymentRepository.findByReservation_Id(newReservation.getId()) 조회는 항상 빈 Optional을 반환하게 되므로, 이 블록은 실행되지 않는 불필요한 데드 코드(Dead Code)이자 불필요한 추가 쿼리를 발생시킵니다.
만약 중복 예약이나 중복 결제 시도를 방지하려는 목적이라면, 새로운 예약을 생성하기 전에 사용자 ID와 시간대(remainId) 등을 기준으로 기존 예약이 존재하는지 먼저 검증하는 로직을 추가하는 것이 올바른 방향입니다. 현재의 중복 제거 로직은 불필요하므로 제거하는 것을 권장합니다.
1 participant
📢 기능 설명
필요시 실행결과 스크린샷 첨부
연결된 issue
연결된 issue를 자동을 닫기 위해 아래 {이슈넘버}를 입력해주세요.
close #{이슈넘버}
✅ 체크리스트